Skip to content

[Backport] Fixed subscribe to newsletter if you already have an account issue#18912

Merged
magento-engcom-team merged 1 commit intomagento:2.2-developfrom
ravi-chandra3197:patch-8952
Jan 2, 2019
Merged

[Backport] Fixed subscribe to newsletter if you already have an account issue#18912
magento-engcom-team merged 1 commit intomagento:2.2-developfrom
ravi-chandra3197:patch-8952

Conversation

@ravi-chandra3197
Copy link
Copy Markdown
Contributor

@ravi-chandra3197 ravi-chandra3197 commented Oct 29, 2018 •
edited
Loading

Description (*)

Fixed subscribe to newsletter if you already have an account issue

Fixed Issues (if relevant)

  1. You can't subscribe to newsletter if you already have an account #8952: You can't subscribe to newsletter if you already have an account

Manual testing scenarios (*)

N/A

Contribution checklist (*)

  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages
  • All new or changed code is covered with unit/integration tests (if applicable)
  • All automated tests passed successfully (all builds on Travis CI are green)

@magento-engcom-team
Copy link
Copy Markdown
Contributor

magento-engcom-team commented Oct 29, 2018

Hi @ravi-chandra3197. Thank you for your contribution
Here is some useful tips how you can test your changes using Magento test environment.
Add the comment under your pull request to deploy test or vanilla Magento instance:

  • @magento-engcom-team give me test instance - deploy test instance based on PR changes
  • @magento-engcom-team give me $VERSION instance - deploy vanilla Magento instance

For more details, please, review the Magento Contributor Assistant documentation

@ravi-chandra3197 ravi-chandra3197 changed the title Fixed subscribe to newsletter if you already have an account issue #8952 Fixed subscribe to newsletter if you already have an account issue Oct 29, 2018
sidolov
sidolov requested changes Oct 29, 2018
View reviewed changes
app/code/Magento/Newsletter/Controller/Subscriber/NewAction.php Outdated
throw new LocalizedException(
__('This email address is already assigned to another user.')
);
if ($this->_customerSession->getCustomerDataObject()->getEmail()) {
Copy link
Copy Markdown
Contributor

@sidolov sidolov Oct 29, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please, combine two if statements to one

@ravi-chandra3197
Copy link
Copy Markdown
Contributor Author

ravi-chandra3197 commented Oct 29, 2018

Hello @sidolov
I have update change as per your suggestion can review it.

@ravi-chandra3197
Copy link
Copy Markdown
Contributor Author

ravi-chandra3197 commented Nov 3, 2018

Hello @sidolov
can you guide how to add the custom test case in magento2 like my Travis fail due to default test case I need to add custom test case so can you guide

@ravi-chandra3197
Copy link
Copy Markdown
Contributor Author

ravi-chandra3197 commented Nov 5, 2018

Hello @sidolov
I have update change as per your suggestion can review it.

@orlangur orlangur self-assigned this Nov 7, 2018
orlangur
orlangur requested changes Nov 7, 2018
View reviewed changes
Copy link
Copy Markdown
Contributor

@orlangur orlangur left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nowadays all changes must be applied to 2.3-develop first (see https://devdocs.magento.com/guides/v2.3/contributor-guide/contributing.html#rules). Please prepare a new pull request, this one should be on hold until the latter is merged.

@ravi-chandra3197 ravi-chandra3197 changed the title Fixed subscribe to newsletter if you already have an account issue [Backport] Fixed subscribe to newsletter if you already have an account issue Nov 12, 2018
@ravi-chandra3197
Copy link
Copy Markdown
Contributor Author

ravi-chandra3197 commented Nov 12, 2018 •
edited
Loading

#19164 Fixed subscribe to newsletter if you already have an account issue #8952 -2.3 develop #19164

@ravi-chandra3197 ravi-chandra3197 mentioned this pull request Nov 12, 2018
Merged
4 tasks
@orlangur
Copy link
Copy Markdown
Contributor

orlangur commented Nov 12, 2018

@ravi-chandra3197 after 2.3 PR is merged, please rewrite history of this branch to reflect it (cherry pick the only meaningful commit).

@ravi-chandra3197
Fixed subscribe to newsletter if you already have an account issue up…
b1fc4a7 
…date changes as per travis change session message
@ravi-chandra3197
Copy link
Copy Markdown
Contributor Author

ravi-chandra3197 commented Dec 12, 2018

Hello, @orlangur I have rewrite branch history with single commit can review it.

@magento-engcom-team magento-engcom-team added this to the Release: 2.2.8 milestone Dec 12, 2018
@magento-engcom-team
Copy link
Copy Markdown
Contributor

magento-engcom-team commented Dec 12, 2018

Hi @orlangur, thank you for the review.
ENGCOM-3680 has been created to process this Pull Request

@magento-engcom-team magento-engcom-team mentioned this pull request Jan 2, 2019
Closed
@magento-engcom-team magento-engcom-team merged commit b1fc4a7 into magento:2.2-develop Jan 2, 2019
magento-engcom-team pushed a commit that referenced this pull request Jan 2, 2019
ENGCOM-3680: [Backport] Fixed subscribe to newsletter if you already …
1f8cbd9 
…have an account issue #18912
@magento-engcom-team
Copy link
Copy Markdown
Contributor

magento-engcom-team commented Jan 2, 2019

Hi @ravi-chandra3197. Thank you for your contribution.
We will aim to release these changes as part of 2.2.8.
Please check the release notes for final confirmation.

Please, consider to port this solution to 2.3 release line.
You may use Porting tool to port commits automatically.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sidolov sidolov sidolov requested changes

@orlangur orlangur orlangur approved these changes

Assignees

@orlangur orlangur

Labels

Component: Newsletter Partner: Krish TechnoLabs partners-contribution Pull Request is created by Magento Partner Progress: accept Release Line: 2.2

Projects

None yet

Milestone

Release: 2.2.8

Development

Successfully merging this pull request may close these issues.

4 participants

@ravi-chandra3197 @magento-engcom-team @orlangur @sidolov